Conversation
* fix: 🐛 docker volume for postgres data * feat: ✨ Abstract bot event listeners to files * fix: 🐛 improve SPDX validation and add verification workflow * feat: ✨ detect existing archivals for fair release status * refactor: ♻️ Add line break after pr badge in issue dashboard * fix: 🐛 add await to db call * refactor: ♻️ Sourcery suggestion * fix(sourcery): 🐛 Edge case with doi format and badge generation
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
Reviewer's GuideRefactors the Probot app into event-specific handlers, enhances FAIR archival and license validation logic (including DOI/Zenodo handling and SPDX/custom license flows), updates the dashboard UI and re-rendering pipeline to work off database-backed metadata, and adds support for the codemeta 2.0 schema in the validator container. Sequence diagram for bot PR lifecycle and dashboard re-rendersequenceDiagram
actor Dev as Developer
participant GH as GitHub
participant App as ProbotApp
participant PRH as pullRequest_handlers
participant DB as PrismaClient
participant ACT as actions_reRenderDashboard
participant REN as renderer_renderIssues
Dev->>GH: Open PR (e.g. LICENSE or metadata PR)
GH->>App: pull_request.opened
App->>PRH: handle pull_request.opened
PRH->>DB: findUnique_installation(repository_id)
alt installation missing or disabled or rate_limited
PRH-->>App: return (ignore)
else valid installation
alt LICENSE PR
PRH->>DB: update licenseRequest.pull_request_url
else METADATA PR
PRH->>DB: update codeMetadata.pull_request_url
end
PRH->>ACT: reRenderDashboard(context, owner, repository, empty_issue_body)
ACT->>DB: fetch licenseRequest, codeMetadata, cwl, readme, contributing, cofc
ACT->>REN: renderIssues(context, owner, repository, emptyRepoFlag, subjectsFromDB)
REN->>DB: update dashboard issue body
end
Dev->>GH: Close or merge PR
GH->>App: pull_request.closed
App->>PRH: handle pull_request.closed
PRH->>DB: clear pull_request_url on licenseRequest or codeMetadata
PRH->>ACT: reRenderDashboard(context, owner, repository, empty_issue_body)
ACT->>REN: renderIssues(...)
REN->>DB: update dashboard issue body (remove PR badge)
PRH->>GH: octokit.git.deleteRef(branch)
Class diagram for new handler and helper modulesclassDiagram
class ProbotApp {
}
class InstallationHandlers {
+registerInstallationHandlers(app, db)
}
class PushHandler {
+registerPushHandler(app, db)
}
class PullRequestHandlers {
+registerPullRequestHandlers(app, db)
}
class IssueHandlers {
+registerIssueHandlers(app, db)
}
class Helpers {
+ISSUE_TITLE
+PR_TITLES
+createEmptyCommitInfo()
}
class ArchivalModule {
+applyArchivalTemplate(context, baseTemplate, repository, owner, subjects)
+extractDOIFromString(value)
+classifyIdentifier(identifier)
+extractIdentifiersFromCodemeta(codemetaContent)
+extractIdentifiersFromCitation(citationContent)
+fetchAndExtractIdentifiers(context, owner, repository)
+prioritizeIdentifiers(identifiers)
+createShieldsLabelFromDOI(doi)
+createZenodoDOIBadge(doi, zenodoId)
+createOtherDOIBadge(doi)
}
class LicenseModule {
+isValidSpdxLicense(licenseId)
+normalizeContent(content)
+validateLicense(license, existingLicense)
+updateLicenseDatabase(repository, license)
+applyLicenseTemplate(baseTemplate, repository, subjects, licenseId, customTitle)
}
ProbotApp --> InstallationHandlers
ProbotApp --> PushHandler
ProbotApp --> PullRequestHandlers
ProbotApp --> IssueHandlers
InstallationHandlers --> Helpers
PushHandler --> Helpers
PullRequestHandlers --> Helpers
IssueHandlers --> Helpers
InstallationHandlers --> ArchivalModule
PushHandler --> ArchivalModule
IssueHandlers --> ArchivalModule
InstallationHandlers --> LicenseModule
PushHandler --> LicenseModule
IssueHandlers --> LicenseModule
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
ui/pages/dashboard/[owner]/[repo]/edit/license.vue, the<n-select>was changed fromv-model:value="licenseId"to:value="licenseId"without wiring an@update:valuehandler, so user selections will no longer updatelicenseIdor triggerupdateLicenseContent; reintroducev-model:valueor add an explicit@update:valueto keep the selection reactive. - The new archival helpers in
bot/compliance-checks/archival/index.jsadd quite a bit of logic; consider moving the identifier/DOI parsing and badge generation into a separate module (e.g.,utils/doi.js) to keep the archival template function focused and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ui/pages/dashboard/[owner]/[repo]/edit/license.vue`, the `<n-select>` was changed from `v-model:value="licenseId"` to `:value="licenseId"` without wiring an `@update:value` handler, so user selections will no longer update `licenseId` or trigger `updateLicenseContent`; reintroduce `v-model:value` or add an explicit `@update:value` to keep the selection reactive.
- The new archival helpers in `bot/compliance-checks/archival/index.js` add quite a bit of logic; consider moving the identifier/DOI parsing and badge generation into a separate module (e.g., `utils/doi.js`) to keep the archival template function focused and easier to maintain.
## Individual Comments
### Comment 1
<location path="bot/index.js" line_range="46-50" />
<code_context>
- });
- }
- });
+ // Register all event handlers
+ registerInstallationHandlers(app, db);
+ registerPushHandler(app, db);
+ registerPullRequestHandlers(app, db);
+ registerIssueHandlers(app, db);
};
</code_context>
<issue_to_address>
**issue (bug_risk):** The `db` variable passed into handler registration is undefined; use `dbInstance` instead.
`dbInstance` is the only database export from `./db.js`, and `db` is not defined in this scope. The calls to `registerInstallationHandlers`, `registerPushHandler`, `registerPullRequestHandlers`, and `registerIssueHandlers` will fail at runtime. Replace `db` with `dbInstance` so each handler receives the actual Prisma client instance.
</issue_to_address>
### Comment 2
<location path="bot/compliance-checks/archival/index.js" line_range="219" />
<code_context>
- const lastVersion = existingZenodoDep.github_tag_name;
- const zenodoId = existingZenodoDep.zenodo_id;
- const zenodoDoi = existingZenodoDep.last_published_zenodo_doi;
- const zenodoDOIBadge = `[](${ZENODO_ENDPOINT}/records/${zenodoId})`;
- baseTemplate += `${archiveTitle} ✔️\n\n***${lastVersion}***${alreadyReleaseText}\n\n${zenodoDOIBadge}\n\nReady to create your next FAIR release? Click the button below:\n\n${releaseBadgeButton}\n\n`;
- }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use the new DOI badge helper to avoid label-encoding issues for stored Zenodo DOIs.
This branch still interpolates `zenodoDoi` directly into the shields.io URL, so a stored `last_published_zenodo_doi` containing `/`, `(`, `)`, spaces, or other special characters could break the badge.
To align with the new helpers and centralize encoding, update this to:
```js
const zenodoDOIBadge = createZenodoDOIBadge(zenodoDoi, zenodoId);
```
Suggested implementation:
```javascript
const lastVersion = existingZenodoDep.github_tag_name;
const zenodoId = existingZenodoDep.zenodo_id;
const zenodoDoi = existingZenodoDep.last_published_zenodo_doi;
const zenodoDOIBadge = createZenodoDOIBadge(zenodoDoi, zenodoId);
baseTemplate += `${archiveTitle} ✔️\n\n***${lastVersion}***${alreadyReleaseText}\n\n${zenodoDOIBadge}\n\nReady to create your next FAIR release? Click the button below:\n\n${releaseBadgeButton}\n\n`;
}
```
1. Ensure `createZenodoDOIBadge` is imported or available in this module, matching how it is used elsewhere in the codebase, e.g. an import like:
`const { createZenodoDOIBadge } = require('./path/to/helpers');`
2. If the helper has a different signature (e.g. expects an object `{ doi, zenodoId }`), adjust the call accordingly to match the existing helper definition.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Thanks for closing this pull request! If you have any further questions, please feel free to open a new issue. We are always happy to help! |
Summary by Sourcery
Refactor GitHub App event handling into dedicated handler modules, enhance archival and license compliance logic, and integrate support for the Codemeta 2.0 schema in the validator and dashboard.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: